Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Савельев Григорий #192

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

luvairo-m
Copy link


public static class ObjectExtensions
{
public static string IntoString<T>(this T instance, Func<StringSerializer<T>, StringSerializer<T>> config)

This comment was marked as resolved.


private int? lineLength;

string ISerializer.Serialize(object instance, int nestingLevel)

This comment was marked as resolved.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я тут если что не имела в виду совсем удалять интерфейс, он в целом логично выглядел)
Но ладно, можно не править)


private string SerializeEnumerable(IEnumerable enumerable, int nestingLevel)
{
const int customOffset = 4;

This comment was marked as resolved.

Comment on lines 133 to 134
if (enumerable is IDictionary dict) FillDictionaryBody();
else FillSequenceBody();

This comment was marked as resolved.

Comment on lines 140 to 147
void FillSequenceBody()
{
foreach (var element in enumerable)
{
var output = (this as ISerializer).Serialize(element, nestingLevel);
stringBuilder.AppendLine($"{previousIndent}{elementOffset}{output.TrimEnd()},");
}
}

This comment was marked as resolved.


public interface ISerializer
{
public string Serialize(object instance, int nestingLevel);

This comment was marked as resolved.

@@ -0,0 +1,6 @@
namespace ObjectPrinting.Contracts;

public interface ISerializer
Copy link

@elizShtol elizShtol Dec 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю сделать ISerializer<T>, Serialize<T>
Думаю, что так будет выглядеть логичнее

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Блин, тут дженерики не отобразились в комменте сначала, сейчас поправила

}

[Test]
public void Serializer_MustBeAbleTo_IgnoreTypes()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обычно вместо MustBeAble пишут просто Should)

namespace StringSerializer.Tests;

[TestFixture]
public class StringSerializerTests

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тесты написаны хорошо!)
Я бы еще добавила тестов на то, что у тебя называется финальными объектами

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Что-то в итоге так и не увидела теста на финальные объекты
Может не очень понятно написала, но смысл такой: написать тест в который будет сериализовать int/double/DateTime...,

@@ -1,14 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk">

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants